Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Utilities: Add the pcfconv utility #25374

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

janso3
Copy link
Contributor

@janso3 janso3 commented Nov 12, 2024

This tool is able to convert an X11 PCF font to Serenity's font format. These pcf files are a fairly common bitmap font format, and with this we are easily able to use many of them.

pcfconv

This tool is able to convert an X11 PCF font to Serenity's font format.
These pcf files are a fairly common bitmap font format, and with this we
are easily able to use many of them.
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Nov 12, 2024
@janso3
Copy link
Contributor Author

janso3 commented Nov 12, 2024

Could also turn this into a parser in LibGfx, not sure what makes more sense.

Copy link
Contributor

@nico nico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure where this belongs either! I suppose we can start here.

Given its X11 heritage, we might want to keep it at arm's length, and promote high quality bundled serenity bitmap fonts -- so maybe this should not move to LibGfx. But not sure.

else
value = TRY(m_stream.read_value<LittleEndian<T>>());

if (!(format & PCF_BIT_MASK))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if does nothing

i32 size;
i32 offset;
};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AssertSize<> (also below)

{
StringBuilder builder;
if (m_properties.contains("WEIGHT_NAME"sv))
builder.append(m_properties.get("WEIGHT_NAME"sv).value().get<String>());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we we validate property types?

m_encoding.max_byte1 = TRY(read<i16>(format));
m_encoding.default_char = TRY(read<i16>(format));

size_t num = (m_encoding.max_char_or_byte2 - m_encoding.min_char_or_byte2 + 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check that min < max?

if (table_index < 0)
return {};

if (table_index >= static_cast<ssize_t>(m_encoding.indices.size()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this happen? Should we error out instead?

auto pcf = TRY(PCFFile::create(buffer));

if (pcf->glyph_size().width() > 32 || pcf->glyph_size().height() > 32) {
outln(stderr, "At this time, glyphs may only be 32px wide");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"at most"

ErrorOr<void> draw_glyph(u16 index, Gfx::GlyphBitmap&) const;
u8 glyph_width(u16 index) const { return m_glyphs.at(index).width; }
u8 baseline() const;
size_t highest_codepoint() const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be larger than u16? The mapping only has 16bit values for min and max afaict.

@nico nico added ⏳ pr-waiting-for-author PR is blocked by feedback / code changes from the author and removed 👀 pr-needs-review PR needs review from a maintainer or community member labels Nov 13, 2024
Copy link

stale bot commented Dec 6, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

@stale stale bot added the stale label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏳ pr-waiting-for-author PR is blocked by feedback / code changes from the author stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants